Skip to content
This repository was archived by the owner on Aug 2, 2022. It is now read-only.

Merge develop into kv-database #8992

Merged
merged 173 commits into from
Apr 23, 2020
Merged

Merge develop into kv-database #8992

merged 173 commits into from
Apr 23, 2020

Conversation

arhag
Copy link
Contributor

@arhag arhag commented Apr 23, 2020

Change Description

This PR merges the develop branch (with the new host function system introduced in PR #8868) into kv-database.

It ports all of the intrinsics introduced in PR #8223 to the new host function system.

The new host function system revealed that the test wast code in the kv_tests/notify unit test was aliasing the key and value spans in calls to the kv_set intrinsic. The test wast code has been modified to avoid aliasing.

This PR also includes fixes to a couple of additional bugs that were introduced in PR #8868 (one in webassembly/preconditions.hpp and one in webassembly/common.hpp):

diff --git a/libraries/chain/include/eosio/chain/webassembly/preconditions.hpp b/libraries/chain/include/eosio/chain/webassembly/preconditions.hpp
index 42eb44ba5..75564b468 100644
--- a/libraries/chain/include/eosio/chain/webassembly/preconditions.hpp
+++ b/libraries/chain/include/eosio/chain/webassembly/preconditions.hpp
@@ -127,7 +127,7 @@ namespace eosio { namespace chain { namespace webassembly {
             }
             if constexpr (is_span_type_v<arg_t> || vm::is_argument_proxy_type_v<arg_t>) {
                eosio::vm::invoke_on<false, eosio::vm::invoke_on_all_t>([&](auto&& narg, auto&&... nrest) {
-                  using nested_arg_t = std::decay_t<decltype(arg)>;
+                  using nested_arg_t = std::decay_t<decltype(narg)>;
                   if constexpr (eosio::vm::is_span_type_v<nested_arg_t> || vm::is_argument_proxy_type_v<arg_t>)
                       EOS_ASSERT(!is_aliasing(detail::to_span(arg), detail::to_span(narg)), wasm_exception, "pointers not allowed to alias");
                }, rest...);
diff --git a/libraries/chain/include/eosio/chain/webassembly/common.hpp b/libraries/chain/include/eosio/chain/webassembly/common.hpp
index 14fa9bbed..393fd385a 100644
--- a/libraries/chain/include/eosio/chain/webassembly/common.hpp
+++ b/libraries/chain/include/eosio/chain/webassembly/common.hpp
@@ -74,7 +74,7 @@ namespace eosio { namespace chain {
       template <typename T>
       auto from_wasm(void* ptr) const
          -> std::enable_if_t< std::is_pointer_v<T>,
-                              vm::argument_proxy<std::remove_pointer_t<T>>> {
+                              vm::argument_proxy<T> > {
          validate_pointer<std::remove_pointer_t<T>>(ptr, 1);
          return {ptr};
       }

I recommend using the following diff to review the relevant changes (excluding the parts that just bring in code changes that were already merged into develop).

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

swatanabe-b1 and others added 25 commits April 21, 2020 16:36
Add nodeos RPC API index, improve nodeos implementation doc, fix link
Avoid legacy for set_action_return_value intrinsic
Avoid aliasing the key and value spans within the kv_set intrinsic calls made in test contracts used for the `kv_tests/notify` unit tests.
@arhag arhag requested a review from swatanabe-b1 April 23, 2020 03:21
@@ -766,7 +766,7 @@ static const char kv_notify_wast[] = R"=====(
(drop (call $kv_it_create (get_local 2) (get_local 0) (i32.const 0) (i32.const 0)))
(drop (call $kv_it_create (get_local 2) (get_local 0) (i32.const 0) (i32.const 0)))
(call $kv_it_destroy (i32.const 2))
(drop (call $kv_set (get_local 2) (get_local 0) (i32.const 0) (i32.const 0) (i32.const 0) (i32.const 1)))
(drop (call $kv_set (get_local 2) (get_local 0) (i32.const 0) (i32.const 0) (i32.const 1) (i32.const 1)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alias check is asymmetric. These should not be considered to alias.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will handle this in kv-database.

@arhag arhag merged commit 48e51f8 into kv-database Apr 23, 2020
@arhag arhag deleted the kv-database-merge branch April 23, 2020 15:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.